Skip to content

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Jul 7, 2024

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

Following @smnandre 's comment (#1937 (comment)), it looks like src/*/.gitignore files reference files that does not exist in the context of a Package.
For example, ignoring .php_cs.cache from a package does not make sense, since this file is created by PHP-CS-Fixer at the repository root.

Now, all the src/*/.gitignore contain at least this list:

/assets/node_modules/
/vendor/
/composer.lock
/phpunit.xml
/.phpunit.result.cache

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Jul 7, 2024
@Kocal Kocal force-pushed the doc/better-gitignores branch from 68d6fdb to 7934d3d Compare July 7, 2024 10:49
@Kocal Kocal changed the title Clean and homogenize src/*/.gitattributes Clean and homogenize src/*/.gitignore Jul 7, 2024
@Kocal Kocal mentioned this pull request Jul 7, 2024
@Kocal Kocal force-pushed the doc/better-gitignores branch 2 times, most recently from d5be769 to cce66e7 Compare July 7, 2024 10:52
@Kocal Kocal force-pushed the doc/better-gitignores branch from cce66e7 to 31df4f5 Compare July 7, 2024 10:52
@Kocal Kocal requested review from kbond and smnandre July 7, 2024 10:53
Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we double-check the .gitattributes in the same pass ?

@smnandre
Copy link
Member

smnandre commented Jul 7, 2024

To me, i think this should be something like

.gitignore

/vendor
/composer.lock
/.phpunit.result.cache

.gitattributes

/.git* export-ignore
/.symfony.bundle.yaml export-ignore
/assets/src export-ignore
/assets/test export-ignore
/phpunit.xml.dist export-ignore
/tests export-ignore

(without the assets line when/if not needed)

Probably also add when necessary

  • vitesst.config (ex: turbo)
  • docker-compose (ex: turbo)
  • phpstan (ex: turbo)

Be careful Turbo has some custom CI ;)

@smnandre
Copy link
Member

So is there still something to agree on here? Can i help you on something @Kocal ?

@Kocal
Copy link
Member Author

Kocal commented Jul 10, 2024

I didn't take a look since, maybe tonight!

@smnandre
Copy link
Member

I don't like adding generated files (phpunit/phpcs/.. cache) ... but if we use CI actions that commit (auto-CS ?) this makes sense.

But i also realised all of those are already ignored in the root .gitignore

.doctor-rst.cache
.php-cs-fixer.cache
node_modules
yarn-error.log
/composer.lock
/vendor

(we may add phpunit.result.cache here)


So, after your explanations, the package folders could contain:

.gitignore

# not sure if here .. maybe in the global gitignore
/.php-cs-fixer.cache 
/.phpunit.result.cache 

# is the root one not enough ?
/assets/node_modules/

/vendor/
/composer.lock
/phpunit.xml

(with aditional files depending on the package content)

.gitattributes

/.gitattributes export-ignore
/.gitignore export-ignore
/.symfony.bundle.yaml export-ignore
/assets/src export-ignore
/assets/test export-ignore
/phpunit.xml.dist export-ignore
/tests export-ignore

(with aditional files depending on the package content)

# And soon
/doc/ export-ignore # as no one want to read raw RST file in their vendor folder
/phpstan.dist.neon export-ignore # if not in the root repo

Side question @Kocal :
Would it be something complicated / with consequences to put the node_module in the root directory of each package ? Or maybe "not advised" ?


So we may only need to update the root .gitignore one for now :)

@Kocal
Copy link
Member Author

Kocal commented Jul 14, 2024

Would it be something complicated / with consequences to put the node_module in the root directory of each package ? Or maybe "not advised" ?

Not complicated, but really not wanted. With this kind of project structure you want to use Yarn workspaces (added in #9 :D)

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

Rebased / clean in ##2243 .. LGTM !

Kocal added a commit that referenced this pull request Oct 6, 2024
) (Kocal, smnandre)

This PR was merged into the 2.x branch.

Discussion
----------

[UX] Clean .gitignore / .gitattributes (rebase + merge #1969)

Finalize #1969 (thak you `@Kocal` !! 💐 )

Commits
-------

ec11e49 Add `/doc` to .gitattributes export-ignore
d9e9caa Clean and homogenize `src/*/.gitignore`
@Kocal Kocal closed this Oct 6, 2024
@Kocal Kocal deleted the doc/better-gitignores branch July 28, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Needs Review Needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants